feature: Customize status bundle_loading_duration_ns

[jwu730-1]

Why the changes in this PR are needed?

This PR enables the customization of the status configuration to set the bundle_loading_duration_ns buckets and resolves issue #6950

What are the changes in this PR?

extend the support of the prometheus config as below

status:
    prometheus: true
    prometheus_config:
      bundle_loading_duration_ns:
        buckets: [1, 1000, 10_000, 1e8]

• inject the prometheus configuration during config parsing
• updated bundleLoadDuration to be customized based on the config
• during reconfig, re-register collector bundleLoadDuration
• added unit test
• small refactoring to make the change more readable

Notes to assist PR review:

Further comments:

[netlify]

✅ Deploy Preview for openpolicyagent ready!

Name	Link
🔨 Latest commit	87d6e6d
🔍 Latest deploy log	https://app.netlify.com/sites/openpolicyagent/deploys/6737c40b6f59ef0008726b95
😎 Deploy Preview	https://deploy-preview-7156--openpolicyagent.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[ashutosh-narkar]

Thanks for the contribution @jwu730-1. One thing I notice in your implementation is that we're allowing users to configure the buckets and then defaulting to ExponentialBuckets. I understand this can be an implementation detail but if we simply allowed the user to configure the exponential bucket parameters (start, factor, count for example) and fallback to the current values, it would help to better understand the changes. This should serve your use-case as well and probably simplify the implementation. WDYT?

[jwu730-1]

Hi Ashutosh, thanks for your comment!

I think it would be flexible to set a customized bucket array. e.g. for our use case, we mostly need rough range e.g. 1s, 10s, 1min 3min (mostly)
Also, this configuration will be consistent with buckets setup as http_request_duration_seconds. I keep the default of ExponentialBuckets to be consistent as before in case someone alredy use that for their opa metrics.

Thanks for the contribution @jwu730-1. One thing I notice in your implementation is that we're allowing users to configure the buckets and then defaulting to ExponentialBuckets. I understand this can be an implementation detail but if we simply allowed the user to configure the exponential bucket parameters (start, factor, count for example) and fallback to the current values, it would help to better understand the changes. This should serve your use-case as well and probably simplify the implementation. WDYT?

[ashutosh-narkar]

I think it would be flexible to set a customized bucket array. e.g. for our use case, we mostly need rough range e.g. 1s, 10s, 1min 3min (mostly)

Can't this be accomplished by setting the parameters of ExponentialBuckets?

[jwu730-1]

if using parameter of ExponentialBuckets, there would be possibly some unused buckets, we would like to eliminate the unused buckets to reduce the metrics size :)

Can't this be accomplished by setting the parameters of ExponentialBuckets?

[ashutosh-narkar]

there would be possibly some unused buckets, we would like to eliminate the unused buckets to reduce the metrics size :)

Thanks for the context. Are you able to quantify this? If it's not significant, then going with the simpler implementation of configuring the exponential bucket parameters should suffice.

[jwu730-1]

Thanks for the context. Are you able to quantify this? If it's not significant, then going with the simpler implementation of configuring the exponential bucket parameters should suffice.

we use the bundle_loading_duration_ns_bucket le as a metric cardinality tag, so we want to reduce the unique values to avoid unnecessary time series.

BTW, I think with exponential bucket parameter, the implementation update is almost the same except

• apply the 2 args instead of assigning the bucket array value)
• use different inject and validate logic

Would love to get comments/suggestions to simplify the implementation, it probably also applies to the current PR. Thanks!

[ashutosh-narkar]

@jwu730-1 the changes seem good. Few comments 👇 . We should also add a test case to exercise the reconfig scenario in the status plugin.

[ashutosh-narkar]

@jwu730-1 the changes look good. Few minor comments. Please squash your commits per the guidelines here and we can get this in. Thanks for working on this!

[ashutosh-narkar]

LGTM. Please squash your commits and we can get this in.

[jwu730-1]

Done. Thank you @ashutosh-narkar!

[ashutosh-narkar]

Thanks for the contribution @jwu730-1!