xmidt-org / talaria

The Xmidt routing agent.
Apache License 2.0
10 stars 20 forks source link

Talaria should provide a metric showing the maxDevices #299

Open schmidtw opened 1 year ago

schmidtw commented 1 year ago

Talaria should provide a metric that shows the configured number of maxDevices so it is easy to see via metrics.

Sachin4403 commented 1 year ago

@johnabass i was going through the code, there i was bit confused about the right location for this metric, Could you please help me with the right location of this metric

cc: @schmidtw

johnabass commented 1 month ago

The maxDevices configuration property isn't exposed via the device.Manager intentionally. All the metrics we keep are implemented through the device.Listener interface. So, to implement this we'd need a few core changes:

(1) A MaxDevices() int method should be exposed in the device.Manager type. That code is in github.com/xmidt-org/webpa-common/device

(2) We need to add support for prometheus function metrics to the github.com/xmidt-org/wepba-common/xmetrics package. Specifically, a NewGaugeFunc(name, func() float64) prometheus.GaugeFunc method is needed.

(3) In github.com/xmidt-org/talaria/main.go, a new function needs to be added that will add this new metric and any future static metrics we want. We can leverage prometheus' gauge function type. Here's some pseudocode:

func newStaticMetrics(m device.Manager, r xmetrics.Registry) (err error) {
    err = r.NewGaugeFunc(
        maxDevicesMetricName, // this can be soft-coded someplace, like a global constant
        func() float64 {
            return float64(m.MaxDevices())
        },
    )

    // future metrics can go here

    return
)

(4) In github.com/xmidt-org/talaria/main.go, add a call to newStaticMetrics after the device manager is created inside the talaria(...) function. Make sure to handle errors in the same way as the rest of that function.

Talaria is currently using go-kit for its metrics, and I'm not sure if go-kit supports function metrics. If it doesn't, we'll have to rethink things.

Sachin4403 commented 1 month ago

@Chaitanyasingla-dt Please do these requested changes and raise an PR for the same.

ChaitanyaSingla commented 3 weeks ago

@schmidtw @johnabass I have raised two pull requests for the changes:

  1. https://github.com/xmidt-org/talaria/pull/454
  2. https://github.com/xmidt-org/webpa-common/pull/1048

cc: @Sachin4403

Sachin4403 commented 3 weeks ago

Hello @johnabass

Please review the Webpa-Common PR, once that is merged and a new version is released then we can incorporate the version in talaria PR

cc: @schmidtw