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

Talaria should provide a metric showing the maxDevices #299

Open
schmidtw opened this issue Mar 16, 2023 · 6 comments
Open

Talaria should provide a metric showing the maxDevices #299

schmidtw opened this issue Mar 16, 2023 · 6 comments

Comments

@schmidtw
Copy link
Member

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

@Sachin4403
Copy link
Contributor

@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
Copy link
Contributor

johnabass commented Jul 18, 2024

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
Copy link
Contributor

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

@Sachin4403
Copy link
Contributor

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

@ChaitanyaSingla
Copy link

Hello @johnabass
Any update regarding the code review?

cc: @schmidtw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants