tibbe/ekg-core

Library for tracking system metrics

ekg-core's Issues

Dynamically creating metrics

Sometimes you want to generate metrics on the fly. Example use cases:

• You don't want to register metrics before hand since you have a lot of them, instead you'd like a fire-and-forget "create this counter if it doesn't exist, and increment it" operation. (Example code). I think this would help for use cases like this, as well.
• Your metric name may be parameterized by some value in your function (this is pretty common)

Currently ekg-core makes it difficult to dynamically create metrics. Reasons:

1. If you try to register a metric that already exists, you'll get a runtime error.
2. There isn't a way of getting a reference to an already created Counter from your Store, you need to track that yourself.

As a workaround to this, for example, @tfausak keeps an IORef of

data Metrics = Metrics
    { metricsCounters :: IORef.IORef (Map.Map Text.Text Counter.Counter)
    , metricsDistributions :: IORef.IORef (Map.Map Text.Text Distribution.Distribution)
    , metricsGauges :: IORef.IORef (Map.Map Text.Text Gauge.Gauge)
    , metricsServer :: Maybe Monitoring.Server
    , metricsStore :: Metrics.Store
    }

in his Blunt package (used for pointfree.info).

Is this a problem? I think so: it's a hassle for users to keep a map of all their metrics, especially when the Store is pretty much doing that already.

Proposed solution

ekg-core provides an API to lookup already registered metrics. Probably a lookupOrCreate type of API should be added as well. The main complication is that ekg-core doesn't actually track the Counters, Distributions, Labels or Gauges you make, it tracks this data:

-- TODO: Rename this to Metric and Metric to SampledMetric.
data MetricSampler = CounterS !(IO Int64)
                   | GaugeS !(IO Int64)
                   | LabelS !(IO T.Text)
                   | DistributionS !(IO Distribution.Stats)

Thoughts? I imagine the choice to track CounterS !(IO Int64) instead of the actual Counter was intentional, so maybe I'm missing something.

Atomic C functions are not atomic.

hs_atomic_read and hs_atomic_write aren't atomic at all on machines without strong memory models (total read and store ordering). To see an example of the kind of havoc this bad assumption can cause, see this (thankfully now fixed) GHC issue https://gitlab.haskell.org/ghc/ghc/-/issues/15449

Here's the aarch64 code that GCC 8.2 yields for these functions: https://godbolt.org/z/giVdtx No barriers or acq/rel lda/sta emitted...

I'm curious why C functions are used for these atomic operations at all. Are these functions really that much faster than atomicModifyIORef?

[Feature Request] Distributions should track percentiles

A nice feature of StatsD's timers is that they track percentiles, in addition to things that Distribution already tracks like the min, max, mean, etc. Having e.g. 99th and 95th percentile data is useful when you know the mean could be biased by outliers. For example:

• At work we track the maximum size of cookies to make sure we aren't getting too close to the 4K limit. Having one user (the maximum) at the limit is very different than the 90th percentile maximum near the limit.
• Distributions tracking time to complete a task might be typically be quite fast, but edge cases might pull the mean upwards (e.g. most users in your social network have 100 friends, a few users have 100,000; most runs of a script are very vast, but running it with verbose logging to debug it might slow it to a crawl).

@tibbe had this to say about the implementation:

Context on why the Distribution metric works as it does: statsd takes the approach of computing percentiles by storing every single data point. This doesn't scale well (e.g. we don't do it this way at Google for that reason). Since I wrote ekg with the Google use case in mind I didn't want to copy this limitation into ekg. The best way I know how to do this is by storing histograms (not the theoretically best approach, but the best engineering approach) and computing approximate percentiles from that. I planned to implement that, but never got around to it. It might be possible to make ekg work with the statsd approach, but it would require reworking the internals (e.g. so that each metric update results in a callback, from which we could send a packet to statsd).

Support base-4.12

I can't update ekg-carbon until ekg-core builds with base-4.12.

Make a release

The current HEAD works with GHC 8.2. The latest release does not. A new release would help people trying out ekg-core with GHC 8.2.

Distribution has N/A variance and mean

I'm seeing a distribution with N/A variance and mean (the json is null). I assume this is due to a division by 0, but I can't see how that's possible. I'm only using the add function which increments the count by 1.

Any idea what might be the cause? Has this been reported before?

Thanks :)

Expand bounds for base + ghc-prim

https://github.com/commercialhaskell/stackage/blob/157cdb59416553a64457082a610704ef9f4ae8ac/build-constraints.yaml#L5865-L5866

This builds fine when manually adding it to the snapshot, so it should just be metadata revision

publish to Hackage for GHC 9 support?

Can a new release including #43 be published to Hackage? Thanks

Enable GC stats collection on demand

Today we require users to run their programs with +RTS -T or compile them with -rtsopts if they want GC stats. Turns out there's a way to enable GC stats during runtime:

http://comments.gmane.org/gmane.comp.lang.haskell.ghc.devel/5281

We should do this whenever the user requests GC stats to be monitored.

Linker error on Win32 — undefined reference to `__sync_fetch_and_add_8' etc.

Hi,
I am unable to use the ekg-core package on Win32. Apparently it relies on GNU atomic intrinsics that are not available in the MinGW bundled with Win32 builds of GHC.

When trying to compile the example program from repo, I'm getting the following output:

C:\Users\mwurb_000\ekg-core\examples>ghc Group.hs
[1 of 1] Compiling Main             ( Group.hs, Group.o )
Linking Group.exe ...
C:\Users\mwurb_000\AppData\Roaming\cabal\i386-windows-ghc-7.8.3\ekg-core-0.1.0.2
/libHSekg-core-0.1.0.2.a(atomic.o):atomic.c:(.text+0x8): undefined reference to
`__sync_fetch_and_add_8'
C:\Users\mwurb_000\AppData\Roaming\cabal\i386-windows-ghc-7.8.3\ekg-core-0.1.0.2
/libHSekg-core-0.1.0.2.a(atomic.o):atomic.c:(.text+0x14): undefined reference to
 `__sync_fetch_and_sub_8'
C:\Users\mwurb_000\AppData\Roaming\cabal\i386-windows-ghc-7.8.3\ekg-core-0.1.0.2
/libHSekg-core-0.1.0.2.a(distrib.o):distrib.c:(.text+0x30): undefined reference
to `__sync_bool_compare_and_swap_8'
collect2: ld returned 1 exit status

GHC 8.2.1 warnings

The GHC.Stats module seems to have been refactored in GHC 8.2.1, so we now get a bunch of warnings of the following type when compiling System.Metrics:

System/Metrics.hs:414:55: warning: [-Wdeprecations]
    In the use of ‘bytesAllocated’ (imported from GHC.Stats):
    Deprecated: "Use RTSStats instead.  This will be removed in GHC 8.4.1"
    |
414 |      [ ("rts.gc.bytes_allocated"          , Counter . Stats.bytesAllocated)
    |                                                       ^^^^^^^^^^^^^^^^^^^^

See this gist for the full log. Looks like we'll have to write a compatibility shim to support future GHC versions.

Allow specifying tags for metrics

OpenTSDB - the time series database used by bosun - uses a different naming convention than EKG. Metrics have an arbitrary name, but they also have a list of key-value pairs (tags). The idea is that you can now have more general metrics such as http.response_time, and then tag data points with specific information such as route=/api/list-foos.

I wonder if EKG should support such a thing, or if I should add support for "post-processing" metrics to ekg-bosun - postProcess :: Name -> (Name, [Tag]), or something to that effect.

Thoughts?

Counter overflow

Hi,

System.Metrics.Counter.Counter is described as "non-negative, monotonically increasing" in the docs, but is an Int64 internally which means that when it overflows it goes negative. Admittedly, 2^63 is quite large, but not unheard-of in the context of system monitoring. The add function can also be passed a negative addend to decrease the counter's value.

I suspect this is just a documentation issue - the "non-negative" bit isn't correct and the "monotonically increasing" bit could be qualified with "except at overflow or if add is passed a negative number". Making it behave like a Word64 would be nice but seems impossible to do without breaking the API.

I'm feeding ekg's statistics into RRD which gracefully handles a counter wraparound at 2^64 but not, apparently, at 2^63.

Cheers,

build failure on ghc881

This package fails to build with ghc881. It looks like the upper bound version of base is too low.

GHC 8.2.1 compilation error: Constructor ‘Stats.GCStats’ does not have the required strict field(s)

Greetings.

I didn't see it on the issue dashboard so here's the error I got when building my library which has ekg-core as a dependency.

[9 of 9] Compiling System.Metrics   ( System/Metrics.hs, dist/build/System/Metrics.o )
System/Metrics.hs:449:16: error:
    • Constructor ‘Stats.GCStats’ does not have the required strict field(s): parBalancedBytesCopied
    • In the expression:
        Stats.GCStats
          {bytesAllocated = 0, numGcs = 0, maxBytesUsed = 0,
           numByteUsageSamples = 0, cumulativeBytesUsed = 0, bytesCopied = 0,
           currentBytesUsed = 0, currentBytesSlop = 0, maxBytesSlop = 0,
           peakMegabytesAllocated = 0, mutatorCpuSeconds = 0,
           mutatorWallSeconds = 0, gcCpuSeconds = 0, gcWallSeconds = 0,
           cpuSeconds = 0, wallSeconds = 0, parTotBytesCopied = 0,
           parMaxBytesCopied = 0, mblocksAllocated = 0}
      In an equation for ‘emptyGCStats’:
          emptyGCStats
            = Stats.GCStats
                {bytesAllocated = 0, numGcs = 0, maxBytesUsed = 0,
                 numByteUsageSamples = 0, cumulativeBytesUsed = 0, bytesCopied = 0,
                 currentBytesUsed = 0, currentBytesSlop = 0, maxBytesSlop = 0,
                 peakMegabytesAllocated = 0, mutatorCpuSeconds = 0,
                 mutatorWallSeconds = 0, gcCpuSeconds = 0, gcWallSeconds = 0,
                 cpuSeconds = 0, wallSeconds = 0, parTotBytesCopied = 0,
                 parMaxBytesCopied = 0, mblocksAllocated = 0}
    |
449 | emptyGCStats = Stats.GCStats
    |                ^^^^^^^^^^^^^...

Regards

Export `Stats(..)`

Having Stats exposed would be useful. Currently it can't be sent over the network unless it's rebuilt from scratch through its accessors, we can't write instances for it and so on.

Allow ghc-prim == 0.10.0

To build with ghc-9.6 I currently need:

allow-newer: ekg-core:ghc-prim

I think this can be fixed with a Hackage metadata edit. @andreasabel.

Follow new naming in GHC.Stats?

There is currently a naming mismatch between EKG and GHC.Stats.

For example, we have

     , ("rts.gc.current_bytes_used"       , Gauge . fromIntegral . Stats.gcdetails_live_bytes . Stats.gc)

so current_bytes_used = gcdetails_live_bytes.

This is due to the 2 years old GHC commit ghc/ghc@24e6594#diff-00007a115377c1a1c96e619128dcb206L236 which did this rename.

Should EKG eventually follow the new naming?

It's weird when some stats have the same names as their GHC.Stats counterparts, but others don't.

RFC: Move all ekg-* packages into a monorepo

Hi @tibbe @ejconlon

How do you feel about moving all four ekg-* packages into a monorepo with a top-level cabal.project (preserving history, of course). This'd simplify CI and people would only need to create one PR for changes like adding new GHC version support that touch multiple ekg packages.

Raise an exception on negative arguments passed to Counter.add

To partly address #2 we should raise an exception if someone passes a negative argument to Counter.add.

`Stats` `min` is always 0

A new Distribution zeros out cMin. https://github.com/tibbe/ekg-core/blob/master/System/Metrics/Distribution.hsc#L99

AFAICT it will aways be zero if the input is positive: https://github.com/tibbe/ekg-core/blob/master/cbits/distrib.c#L24

I think cMin would be more useful it was initialized to the largest Double. Then it would show the smallest value seen if a single data point was added.

Cannot build with GHC 7.10.1

It looks like this is fixed in HEAD, but I'm creating an issue so I can be notified when this makes it to Hackage. Then I can update my libraries (ekg-bosun, etc).

Build failure with GHC HEAD

System/Metrics.hs:448:16: error:
    • Constructor ‘Stats.GCStats’ does not have the required strict field(s): mblocksAllocated
    • In the expression:
        Stats.GCStats
          {bytesAllocated = 0, numGcs = 0, maxBytesUsed = 0,
           numByteUsageSamples = 0, cumulativeBytesUsed = 0, bytesCopied = 0,
           currentBytesUsed = 0, currentBytesSlop = 0, maxBytesSlop = 0,
           peakMegabytesAllocated = 0, mutatorCpuSeconds = 0,
           mutatorWallSeconds = 0, gcCpuSeconds = 0, gcWallSeconds = 0,
           cpuSeconds = 0, wallSeconds = 0, parTotBytesCopied = 0,
           parMaxBytesCopied = 0}
      In an equation for ‘emptyGCStats’:
          emptyGCStats
            = Stats.GCStats
                {bytesAllocated = 0, numGcs = 0, maxBytesUsed = 0,
                 numByteUsageSamples = 0, cumulativeBytesUsed = 0, bytesCopied = 0,
                 currentBytesUsed = 0, currentBytesSlop = 0, maxBytesSlop = 0,
                 peakMegabytesAllocated = 0, mutatorCpuSeconds = 0,
                 mutatorWallSeconds = 0, gcCpuSeconds = 0, gcWallSeconds = 0,
                 cpuSeconds = 0, wallSeconds = 0, parTotBytesCopied = 0,
                 parMaxBytesCopied = 0}