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

Enable ability to overwrite mbean attribute value #901

Closed
akats7 opened this issue May 31, 2023 · 9 comments · Fixed by #960
Closed

Enable ability to overwrite mbean attribute value #901

akats7 opened this issue May 31, 2023 · 9 comments · Fixed by #960
Assignees
Labels
component: jmx-metrics help wanted Extra attention is needed type: enhancement New feature or request

Comments

@akats7
Copy link
Contributor

akats7 commented May 31, 2023

Component(s)

jmx-metrics

Is your feature request related to a problem? Please describe.

We have a use case where we would like to create metrics for string attributes, it makes sense to not provide a string callback, it would be useful to be able to force a value in the instrument helper (just set the value equal to '1') or even better it would be if the InstrumentHelper also accepted a callback/closure that allows you to build logic to set the metric value to a long, double, or counter, when the mbean attribute itself is a different type.

Describe the solution you'd like

I would like the InstrumentHelper constructor to accept a closure or callback function, that if set will be used to update the metric value based on the logic in the closure

e.g.,

def metric = otel.mbeans("RANDOM_MBEAN")
otel.instrument(metric, "metric_based_off_string",
        "description", "by",
        "string-metric",  checkValue ,otel.&longValueCallback)
def checkValue = { value ->
  if (value == "running") {
    return 1
  } else {
      return 0 
  }
} 

Describe alternatives you've considered

No response

Additional context

No response

@akats7
Copy link
Contributor Author

akats7 commented Jun 5, 2023

Hi @trask, just wanted to follow up

@trask
Copy link
Member

trask commented Jun 5, 2023

cc: jmx-metrics component owners @breedx-splk @Mrod1598 @rmfitzpatrick @dehaansa

@dehaansa
Copy link
Contributor

dehaansa commented Jun 6, 2023

I don't see any issues with supporting this behavior, but I don't believe I will personally have time to implement it very soon.

@akats7
Copy link
Contributor Author

akats7 commented Jun 6, 2023

I don't see any issues with supporting this behavior, but I don't believe I will personally have time to implement it very soon.

@dehaansa I'd be willing to make the contribution

@breedx-splk breedx-splk added type: enhancement New feature or request help wanted Extra attention is needed labels Jun 7, 2023
@breedx-splk
Copy link
Contributor

breedx-splk commented Jun 7, 2023

@akats7 Sounds like string -> numeric isn't really the core goal, it sounds like a more general-purpose transformation mechanism, which is powerful and useful.

Please feel free to mention me on the PR and I'll review. Thanks!

@rmfitzpatrick
Copy link
Contributor

This sounds like a helpful idea to increase the usable mbean attributes but for the implementation I worry about binding to the instrument helper instead of the mbean one. The overloading is already approaching unreasonable territory for the instrument helpers and something like:

def someBean = otel.mbean(
  "SomeMBean", ["CustomAttrFromString": { mbean -> (mbean.SourceAttr == "running") ? 1 : 0 }]
)
otel.instrument(someBean, "my-metric", "CustomAttrFromString", otel.&longUpDownCounterCallback)

would solve the problem and be able to extend to other instrument calls.

@akats7
Copy link
Contributor Author

akats7 commented Jun 9, 2023

Hey @rmfitzpatrick, thanks for the reply. That makes sense, but it does seem to kind of mix concerns between the mbean and the instrument logic, where you're essentially setting the instrumentation scope when you create the mbean helper. However, if you're more comfortable with that approach, I can implement it that way as well.

@dehaansa @breedx-splk

@dehaansa
Copy link
Contributor

dehaansa commented Jun 9, 2023

I would agree that rmfitzpatrick's proposal would reduce the necessary added complexity to InstrumentHelper (an already very complex bit of functionality).

Adding the behavior to the MBeanHelper should just require a new attribute/constructor, and a relatively simple modification to the getAttribute/getAttributes functions, which does seem cleaner.

@akats7
Copy link
Contributor Author

akats7 commented Jun 9, 2023

I would agree that rmfitzpatrick's proposal would reduce the necessary added complexity to InstrumentHelper (an already very complex bit of functionality).

Adding the behavior to the MBeanHelper should just require a new attribute/constructor, and a relatively simple modification to the getAttribute/getAttributes functions, which does seem cleaner.

sounds good, @trask can I please be assigned this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: jmx-metrics help wanted Extra attention is needed type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants