apple / swift-metrics

Metrics API for Swift
https://apple.github.io/swift-metrics/
Apache License 2.0
643 stars 59 forks source link

Export default process & runtime metrics #53

Open MrLotU opened 4 years ago

MrLotU commented 4 years ago

From the Prometheus client library spec, Client libraries SHOULD offer what they can of the Standard exports, documented below.

Metric name Help string Unit
process_cpu_seconds_total Total user and system CPU time spent in seconds. seconds
process_open_fds Number of open file descriptors. file descriptors
process_max_fds Maximum number of open file descriptors. file descriptors
process_virtual_memory_bytes Virtual memory size in bytes. bytes
process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes. bytes
process_resident_memory_bytes Resident memory size in bytes. bytes
process_heap_bytes Process heap size in bytes. bytes
process_start_time_seconds Start time of the process since unix epoch in seconds. seconds

I think these metrics would be useful to have, also outside of Prometheus.

My proposal is to implement these metrics into swift-metrics directly, with the option of switching them off.

Possible API would be:

MetricsSystem.bootstrap(_ factory: MetricsFactory, provideSystemMetrics: Bool = true)

As a note I'd like to add I'm not 100% sure which of the above metrics are feasible and possible to get a hold of in Swift, so some experimentation/research might be needed there.

tomerd commented 4 years ago

this is a cool idea @MrLotU, would you like to take on proposing an implementation?

ktoso commented 4 years ago

Sounds good, I suppose this could be done as extra module like "SystemMetrics" and added via MetricsSystem.bootstrapSystemMetrics, so it's not necessarily part of core?

Another tricky bit is labels -- this keeps coming up to be honest. I.e. one systems like to report like system.bla.bla while in others . is illegal... What I do in my libraries where I emit metrics is offering the segmentSeparator to be configurable. we migth need to do the same here, as prom wants _ but others may need .... OR we make all labels configurable, could also be good hm

Gathering those may need some execution context? 🤔

Would be nice to figure this out and be able to offer it! Looking forward to more details :)

tomerd commented 4 years ago

Sounds good, I suppose this could be done as extra module like "SystemMetrics" and added via MetricsSystem.bootstrapSystemMetrics, so it's not necessarily part of core?

I think separate module sounds good. not sure how to exactly tie it into bootstrap, so API ideas would be great to discuss

Another tricky bit is labels -- this keeps coming up to be honest. I.e. one systems like to report like system.bla.bla while in others . is illegal... What I do in my libraries where I emit metrics is offering the segmentSeparator to be configurable. we migth need to do the same here, as prom wants _ but others may need .... OR we make all labels configurable, could also be good hm

imo, the restriction logic on labels belong with the collector, not the emitter. iow collectors for backends that are sensitive to certain characters (like Prometheus and dots) should sanitize before sending to the backend. if we put this kind of logic on the generic emission API we would need to worry about all the potential disallowed characters in all the possible backends and this is not scalable. happy to consider suggestions that show otherwise.

Would be nice to figure this out and be able to offer it! Looking forward to more details :)

+1 me too, this would be a great addition imo

MrLotU commented 4 years ago

I'll get some work in over the weekend. Excited to see what we can do here 😄

ktoso commented 4 years ago

imo, the restriction logic on labels belong with the collector, not the emitter. iow collectors for backends that are sensitive to certain characters (like Prometheus and dots) should sanitize before sending to the backend.

Hmm, that is a fair point -- we could indeed handle this in the Prom library, by optionally providing some label replacements chars... This way users of apps who pick prom know "i have a lib which does use . but I use prometheus, so it should be _" or similar. Does indeed sound like the right place to put it... Doing it in my library which should have been agnostic to what backend we emit to felt kind of backwards -- I'll open a ticket there: https://github.com/MrLotU/SwiftPrometheus/issues/29